fix(action menu): keyboard accessibility omnibus #5031

nikkimk · 2025-01-16T21:42:58Z

Action menu items are not reading for screen readers.

Description

Action menu should be using a roving tabindex not aria-activedescendant because of cross-root ARIA limitations as well as lack of iOS support.

The sp-menu that action menu uses was refactored to use a roving tabindex, and the numpad keys fix that was made in action menu are now applied to the focus group controller which the roving tabindex controller uses.

Related issue(s)

Motivation and context

VoiceOver could not read the menu items when navigated via keyboard because of the cross-root aria issues above. Using the same roving tabindex controller that other components in our repo use, allows us to ensure roving tabindex and keyboard navigation is accessible and consistent across all components.

How has this been tested?

By default a Menu should follow the Navigation Menu Example from the WAI ARIA APG.
Menus with selects and menu groups should function like the menu groups in the Editor Menubar Example from the WAI ARIA APG
Please note that there will be VRT differences where open menus that didn't used to have focus now do. Menus that are opened via keyboard are supposed to set focus on a menu item.

Does screenreader read menuitems? (resolves #4556 and without regression on #3751)

Go to https://nikkimk-fix-menu-a11y--spectrum-web-components.netlify.app/components/action-menu/#sizes
Open Voice Over
Tab to the More Actions... menu button.
Press Down Arrow repeatedly to scroll through the menu items
Press Up Arrow repeatedly to scroll through the menu items
Press Numpad Down Arrow repeatedly to and scroll through the menu items
Press Numpad Up Arrow repeatedly to and scroll through the menu items
Screenreader should read menu items

Can you use a screenreader to click a menuitem? (resolves #4997)

Go to https://nikkimk-fix-menu-a11y--spectrum-web-components.netlify.app/components/action-menu/#sizes
Open Voice Over
Tab to the menuitem.
Tab focus to the button.
Press CRTL+Option+Space to click the link.
Link should be activated.

Does keyboard navigation of menuitems work as it should? (closes #4557)

Go to this WAI ARIA APG Menu Button example.
Click on the Actions menu button.
Notice that the menu opens and focus is on the first item.
Press Down Arrow.
Notice that the focus is on the second item.
Now go to https://nikkimk-fix-menu-a11y--spectrum-web-components.netlify.app/storybook/iframe.html?args=&id=action-menu--default&viewMode=story
Click on the More Actions... menu button.
Notice that the menu opens and focus is on the first item.
Press Down Arrow.
Notice that the focus is on the second item.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
[] Breaking change (fix or feature that would cause existing functionality to change) _This change will affect combobox, but combobox was already inaccessible. See RFC for Accessible Menu navigation for more information. _
Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
If my change required a change to the documentation, I have updated the documentation in this pull request.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

…endant

changeset-bot · 2025-01-16T21:43:02Z

🦋 Changeset detected

Latest commit: d163634

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2025-01-16T21:45:03Z

Branch preview

Review the following VRT differences

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

github-actions · 2025-01-16T21:47:11Z

Tachometer results

Currently, no packages are changed by this PR...

github-actions · 2025-01-16T21:49:01Z

Lighthouse scores

Category	Latest (report)	Main (report)	Branch (report)
Performance	0.99	0.98	0.99
Accessibility	1	1	1
Best Practices	1	1	1
SEO	1	0.92	0.92
PWA	1	1	1

What is this?

Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on main ("Main"). Higher scores are better, but note that the SEO scores on Netlify URLs are artifically constrained to 0.92.

Transfer Size

Category	Latest	Main	Branch
Total	249.755 kB	234.757 kB	234.604 kB 🏆
Scripts	62.159 kB	54.43 kB	54.405 kB 🏆
Stylesheet	51.511 kB	45.761 kB 🏆	45.774 kB
Document	6.223 kB	5.502 kB	5.462 kB 🏆
Font	127.012 kB	126.653 kB	126.613 kB 🏆

Request Count

Category	Latest	Main	Branch
Total	52	52	52
Scripts	41	41	41
Stylesheet	5	5	5
Document	1	1	1
Font	2	2	2

coveralls · 2025-01-16T21:52:04Z

Pull Request Test Coverage Report for Build 13318379084

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

446 of 491 (90.84%) changed or added relevant lines in 10 files are covered.
25 unchanged lines in 6 files lost coverage.
Overall coverage decreased (-0.2%) to 97.993%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
tools/reactive-controllers/src/FocusGroup.ts	24	25	96.0%
packages/picker/src/MobileController.ts	3	5	60.0%
packages/picker/src/Picker.ts	103	107	96.26%
packages/menu/src/Menu.ts	163	171	95.32%
packages/menu/src/MenuItem.ts	122	136	89.71%
packages/action-menu/src/ActionMenu.ts	12	28	42.86%

Files with Coverage Reduction	New Missed Lines	%
packages/picker/src/InteractionController.ts	2	94.07%
packages/tabs/src/Tabs.ts	2	98.34%
packages/picker/src/DesktopController.ts	2	90.83%
tools/shared/src/focusable.ts	2	99.32%
packages/menu/src/Menu.ts	8	94.46%
packages/picker/src/Picker.ts	9	97.21%

Totals
Change from base Build 13290469074:	-0.2%
Covered Lines:	33228
Relevant Lines:	33714

💛 - Coveralls

…child WAI ARIA APG

…d nav

…nikkimk/fix-menu-a11y

nikkimk · 2025-02-11T21:15:18Z

packages/action-menu/test/index.ts

@@ -436,56 +436,6 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {

            expect(firstRect).to.deep.equal(secondRect);
        });
-        it('opens and selects in a single pointer button interaction', async () => {


Menu hover was removed for better a11y experience, so this test is no longer needed

…nikkimk/fix-menu-a11y

…emporary debugging

…-components into nikkimk/fix-menu-a11y

… selected

nikkimk · 2025-02-12T17:48:02Z

packages/action-menu/test/action-menu-groups.test.ts

@@ -91,12 +90,7 @@ describe('Action Menu - Groups', () => {
            press: 'ArrowDown',
        });
        await opened;
-        expect(el.open).to.be.true;


When opened, menu should focus on the first selected item, so we don't need to ArrowUp to focus on it

…nikkimk/fix-menu-a11y

…bobox solution is implemented

…using controllers are reviewed for a11y,

nikkimk · 2025-02-13T17:40:19Z

tools/reactive-controllers/src/RovingTabindex.ts

@@ -45,18 +45,17 @@ export class RovingTabindexController<
    }

    manageTabindexes(): void {
-        if (this.focused) {
+        if (this.focused && !this.delegatesFocus) {


Ideally we want to look at the components that use these controllers to see if they are following WAI ARIA APG for keyboard nav, but in the meantime, so we don't have scope-creep on this PR, I'm allowing existing components to still follow this pattern.

nikkimk · 2025-02-13T17:41:15Z

packages/tabs/test/tabs.test.ts

@@ -353,48 +353,6 @@ describe('Tabs', () => {
        await elementUpdated(el);
        expect(el.selected).to.be.equal('first');
    });
-    it('prevents [tabindex=0] while `focusin`', async () => {


Tab panels should not have a tabindex. This test is failing but is not needed.

nikkimk · 2025-02-13T17:42:15Z

packages/combobox/test/combobox-a11y.test.ts

@@ -102,55 +98,26 @@ describe('Combobox accessibility', () => {
        )) as unknown as AccessibleNamedNode & {
            children: AccessibleNamedNode[];
        };
-
-        // WebKit doesn't currently associate the `name` via the accessibility tree.


The time to get rid of the escape hatch is now.

… other components using controllers are reviewed for a11y,

nikkimk added 4 commits January 16, 2025 16:06

fix(menu): added rovingTabindexController and removed aria-activedesc…

91b72f7

…endant

fix(menu): fixes firefox keyboard navigation and call stack issues

adc0d26

test(menu): updates menu test based on changes

97d88c3

test(menu): adds click test for accessibility

d10ed4c

nikkimk linked an issue Jan 16, 2025 that may be closed by this pull request

[Bug][A11y]: Action-Menu menu items doesnt read in screen-readers #4556

Open

1 task

nikkimk added 20 commits January 17, 2025 09:14

test(menu): menu itself should delegate focus to active item

cb74cba

fix(menu): fixed menu group navigation according WAI ARIA APG

3876c36

fix(reactive-controllers): complex element delegates focus to active …

350e9ba

…child WAI ARIA APG

fix(menu): components should delegate focus

b7c750f

fix(menu): update keyboard nav to match WAI ARIA APG

68a298d

fix(menu): updated docs to reflect preferred accessible method

63e1dab

fix(menu): selection and keyboard navigation according to APG

b6ee246

test(menu): updated to recommended a11y behavior from APG

ef99ebb

docs(menu): reverted docs but fixed typos

eb7e539

fix(menu): removed unused map

36cc4a2

test(menu): updated test to match changed group id

9f5f16c

test(menu): keyboard selection is fixed

01be995

test(menu): updated to reflect WAI ARIA APG

c5a9514

fix(picker): picker should delegate focus

a949c61

fix(action-menu): fixed a11y error message

62147ea

fix(picker): according to APG ArrowDown should focus on first item

8211077

test(action-menu): updated tests for focus delegation and APG keyboar…

66eb2d8

…d nav

fix(picker): ensure overlay is open before setting focus

7f6b363

fix(menu): removed extra focus

d4f7f93

Merge branch 'main' of github.com:adobe/spectrum-web-components into …

291231f

…nikkimk/fix-menu-a11y

nikkimk added 6 commits February 6, 2025 14:15

Merge branch 'main' of github.com:adobe/spectrum-web-components into …

d4ab737

…nikkimk/fix-menu-a11y

test(tray): debugging VO/iOS tray

b764a9e

Merge branch 'main' of github.com:adobe/spectrum-web-components into …

004a093

…nikkimk/fix-menu-a11y

fix(menu): resolves submenu tests

82406d5

fix(menu): added some temporary debugging logic and comments

8d56d22

test(action-menu): removed test that no longer applies to a11y

ff9f716

nikkimk commented Feb 11, 2025

View reviewed changes

nikkimk added 4 commits February 12, 2025 09:58

Merge branch 'main' of github.com:adobe/spectrum-web-components into …

5155cca

…nikkimk/fix-menu-a11y

fix(action-menu): fixed menu closing issue, added docs, and removed t…

5d36670

…emporary debugging

Merge branch 'nikkimk/fix-menu-a11y' of github.com:adobe/spectrum-web…

b367419

…-components into nikkimk/fix-menu-a11y

test(action-menu): fixed test based on focus behavior when an item is…

5d81d96

… selected

nikkimk commented Feb 12, 2025

View reviewed changes

nikkimk added 13 commits February 12, 2025 12:52

Merge branch 'main' of github.com:adobe/spectrum-web-components into …

c3582d5

…nikkimk/fix-menu-a11y

merge: merge from main

dfc14ec

merge: merge from main

43a97b8

test(combobox): removing aria-activedescendant references until a com…

10428d1

…bobox solution is implemented

test(combobox): removing aria-activedescendant references until a com…

6c7d3df

…bobox solution is implemented

test(combobox): added skipped tests back

9f34dbb

fix(combobox): resolves timing issues with updated menu

e513273

test(combobox): updated test docs

94e7f49

chore(combobox): removed debugging

b66d51e

docs(combobox): updated webkit escape hatches

13e4524

test(combobox): removed isWebkit since escape hatch is not needed

87f1045

test(tabs): removed test that does not follow WAI ARIA APG

bf0ea21

fix(controllers): delegates focus setting until all other components …

59d39a1

…using controllers are reviewed for a11y,

nikkimk commented Feb 13, 2025

View reviewed changes

nikkimk added 2 commits February 13, 2025 15:26

fix(controllers): fix(controllers): delegates focus setting until all…

8634b69

… other components using controllers are reviewed for a11y,

test(action-menu): fixed typo

d163634

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(action menu): keyboard accessibility omnibus #5031

fix(action menu): keyboard accessibility omnibus #5031

nikkimk commented Jan 16, 2025 •

edited

Loading

changeset-bot bot commented Jan 16, 2025 •

edited

Loading

github-actions bot commented Jan 16, 2025

github-actions bot commented Jan 16, 2025

github-actions bot commented Jan 16, 2025 •

edited

Loading

coveralls commented Jan 16, 2025 •

edited

Loading

nikkimk Feb 11, 2025

nikkimk Feb 12, 2025

nikkimk Feb 13, 2025

nikkimk Feb 13, 2025

nikkimk Feb 13, 2025

fix(action menu): keyboard accessibility omnibus #5031

Are you sure you want to change the base?

fix(action menu): keyboard accessibility omnibus #5031

Conversation

nikkimk commented Jan 16, 2025 • edited Loading

Description

Related issue(s)

Motivation and context

How has this been tested?

Does screenreader read menuitems? (resolves #4556 and without regression on #3751)

Can you use a screenreader to click a menuitem? (resolves #4997)

Does keyboard navigation of menuitems work as it should? (closes #4557)

Types of changes

Checklist

Best practices

changeset-bot bot commented Jan 16, 2025 • edited Loading

🦋 Changeset detected

github-actions bot commented Jan 16, 2025

Branch preview

Review the following VRT differences

github-actions bot commented Jan 16, 2025

Tachometer results

github-actions bot commented Jan 16, 2025 • edited Loading

Lighthouse scores

Transfer Size

Request Count

coveralls commented Jan 16, 2025 • edited Loading

Pull Request Test Coverage Report for Build 13318379084

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

nikkimk Feb 11, 2025

Choose a reason for hiding this comment

nikkimk Feb 12, 2025

Choose a reason for hiding this comment

nikkimk Feb 13, 2025

Choose a reason for hiding this comment

nikkimk Feb 13, 2025

Choose a reason for hiding this comment

nikkimk Feb 13, 2025

Choose a reason for hiding this comment

nikkimk commented Jan 16, 2025 •

edited

Loading

changeset-bot bot commented Jan 16, 2025 •

edited

Loading

github-actions bot commented Jan 16, 2025 •

edited

Loading

coveralls commented Jan 16, 2025 •

edited

Loading